fix: return 400 for JSON nested beyond the parser's stack depth (#123) - #128
Merged
Merged
Conversation
Root cause: V8 applies a JSON.parse reviver recursively, one stack frame per nesting level. The enqueue reviver (unsupported-number check) made bodies nested ~3,100+ levels deep throw RangeError: Maximum call stack size exceeded. enqueueErrorResponse only maps SyntaxError, so the RangeError escaped as an uncaught 500. Plain JSON.parse and JSON.stringify both handle 100,000+ levels, so the parse step was the only point of failure. parseJsonBody now converts a RangeError raised by JSON.parse into a SyntaxError. The request gets the existing 400 "Invalid JSON" response and the Queue is not changed. The catch covers only the parse call, so RangeErrors from anywhere else still surface as 500s. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Throwing `new SyntaxError` added a module dependency and pushed handler.ts to the CouplingBetweenObjects limit (13) in the production quality gate. A local JsonNestingTooDeepError, mapped to the same 400 "Invalid JSON" response, follows the existing UnsupportedNumberError pattern and keeps coupling at 12. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The atomic-snapshot rewrite in #122 left persist.ts at 78-79% on Stryker, failing the per-file threshold for any PR that touches tests. Kill the surviving mutants through the public FileStore interface: full truncation of stale temp content, rethrowing unusable-temp-path errors, temp cleanup on failed rename, and multi-byte reassembly across 4096-byte read boundaries.
jonbaldie
added a commit
that referenced
this pull request
Sep 22, 2026
* fix: make rate limiter checks independent of window size (#125) isAllowed() filtered the caller's full timestamp window on every call, including denied requests and pre-auth traffic, so per-request cost grew linearly with RATE_LIMIT_REQUESTS and collapsed quadratically under load. Timestamps are sorted ascending, so stale entries form a prefix: take an O(1) fast path when the newest is fresh, otherwise binary search the first fresh entry (O(log n)) and count the window without filtering or copying. Drop the stale prefix only once it dominates the array so the copy stays amortized O(1) per recorded request. All-stale entries are still removed, and periodic cleanup and eviction semantics are unchanged. * test: restore persist.ts mutation coverage below the 80% gate The atomic-snapshot rewrite in #122 left persist.ts at 78-79% on Stryker, failing the per-file threshold for any PR that touches tests. Kill the surviving mutants through the public FileStore interface: full truncation of stale temp content, rethrowing unusable-temp-path errors, temp cleanup on failed rename, and multi-byte reassembly across 4096-byte read boundaries. * refactor: extract firstFreshIndex to keep isAllowed under complexity gate * fix: return 400 for JSON nested beyond the parser's stack depth (#123) (#128) * fix: return 400 for JSON nested beyond the parser's stack depth (#123) Root cause: V8 applies a JSON.parse reviver recursively, one stack frame per nesting level. The enqueue reviver (unsupported-number check) made bodies nested ~3,100+ levels deep throw RangeError: Maximum call stack size exceeded. enqueueErrorResponse only maps SyntaxError, so the RangeError escaped as an uncaught 500. Plain JSON.parse and JSON.stringify both handle 100,000+ levels, so the parse step was the only point of failure. parseJsonBody now converts a RangeError raised by JSON.parse into a SyntaxError. The request gets the existing 400 "Invalid JSON" response and the Queue is not changed. The catch covers only the parse call, so RangeErrors from anywhere else still surface as 500s. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor: map parser depth overflow via a local error type Throwing `new SyntaxError` added a module dependency and pushed handler.ts to the CouplingBetweenObjects limit (13) in the production quality gate. A local JsonNestingTooDeepError, mapped to the same 400 "Invalid JSON" response, follows the existing UnsupportedNumberError pattern and keeps coupling at 12. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test: restore persist.ts mutation coverage below the 80% gate The atomic-snapshot rewrite in #122 left persist.ts at 78-79% on Stryker, failing the per-file threshold for any PR that touches tests. Kill the surviving mutants through the public FileStore interface: full truncation of stale temp content, rethrowing unusable-temp-path errors, temp cleanup on failed rename, and multi-byte reassembly across 4096-byte read boundaries. --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> * fix: reject invalid UTF-8 enqueue bodies (#116) (#133) Decode request bytes with a fatal UTF-8 decoder so malformed JSON strings return the existing 400 Invalid JSON response instead of being replaced with U+FFFD. Add HTTP seam coverage for rejection, no enqueue, and valid UTF-8 round trips. * fix: avoid per-value JSON reviver overhead (#134) Parse request bodies natively, then scan the original JSON source to validate number literals. This preserves exact-number rejection while avoiding a reviver callback for every value in number-dense payloads. Add public handler regressions for exact integers, nested metadata, JSON strings, and the explicit nesting limit. --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #123
Summary
V8 applies a
JSON.parsereviver recursively, one stack frame per nesting level. The enqueue reviver (the unsupported-number check) overflows at about 3,100 levels and throwsRangeError.enqueueErrorResponseonly mapsSyntaxError, so theRangeErrorescaped and the server returned a 500. PlainJSON.parseandJSON.stringifyboth handle 100,000+ levels, so parsing is the only step that fails.The catch covers only the
JSON.parsecall, so aRangeErrorfrom anywhere else still returns a 500.Evidence
Requestto the handler with a 4,000-level nested array givesRangeError: Maximum call stack size exceeded, and the handler logs status 500. The new regression test fails:enqueue of JSON nested beyond the parser's stack depth ... FAILED — RangeError: Maximum call stack size exceeded327 passed | 0 failed. Local Mutasaurus:handler.ts 96% (22/23), all files ≥80%.npm run quality:productionpasses. The local error type keeps handler coupling at 12;new SyntaxErrorpushed it to the limit of 13.deno run main.ts+ curl):Merge Danger
Door: two-way.
Blast Radius: enqueue parse-error mapping only. Bodies that used to return an uncaught 500 now return 400
Invalid JSON. Every other status and message is unchanged.🤖 Generated with Claude Code